Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drivers/st77xx: implement initialization #19827

Merged

Conversation

gschorcht
Copy link
Contributor

@gschorcht gschorcht commented Jul 13, 2023

Contribution description

This PR implements correct initialization for ST7735, ST7789 and ST7796. A number of configuration parameters are exposed via Kconfig. The user can decide whether to use custom configuration parameters or reset defaults.

To be compilable, the PR includes PR #19824 and PR #19825

Testing procedure

  • Green CI
  • tests/drivers/disp_dev and tests/drivers/st77xx should still work for all boards using a ST77xx display.
  • The PR was already tested for these tests for:
    • adafruit-pybadge
    • esp32s2-lilygo-ttgo-t8
    • esp32s3-usb-otg
    • sipeed-longan-nano

Issues/PRs references

Depends on #19825
Fixes #19818

@github-actions github-actions bot added Area: doc Area: Documentation Area: tests Area: tests and testing framework Area: drivers Area: Device drivers Area: boards Area: Board ports Area: sys Area: System Area: Kconfig Area: Kconfig integration labels Jul 13, 2023
@gschorcht gschorcht added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jul 13, 2023
@riot-ci
Copy link

riot-ci commented Jul 13, 2023

Murdock results

✔️ PASSED

7599ce7 drivers/st77xx: expose configuration parameters in Kconfig

Success Failures Total Runtime
7968 0 7968 14m:50s

Artifacts

@gschorcht gschorcht force-pushed the drivers/st77xx_fix_init_single_module branch 2 times, most recently from 8278df3 to de203bc Compare July 13, 2023 23:40
@benpicco benpicco added the State: waiting for other PR State: The PR requires another PR to be merged first label Jul 14, 2023
Copy link
Contributor

@MrKevinWeiss MrKevinWeiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some quick things, I can do a more in depth review later in the week.

boards/adafruit-pybadge/Makefile.dep Outdated Show resolved Hide resolved
boards/esp32s2-lilygo-ttgo-t8/Makefile.dep Outdated Show resolved Hide resolved
boards/esp32s3-usb-otg/Makefile.dep Outdated Show resolved Hide resolved
boards/sipeed-longan-nano/Makefile.dep Outdated Show resolved Hide resolved
drivers/Makefile.dep Outdated Show resolved Hide resolved
drivers/st77xx/Kconfig Outdated Show resolved Hide resolved
@gschorcht gschorcht force-pushed the drivers/st77xx_fix_init_single_module branch 3 times, most recently from 60b7abc to 6b81b5f Compare July 24, 2023 12:53
@gschorcht gschorcht force-pushed the drivers/st77xx_fix_init_single_module branch from 6b81b5f to bbfa0bd Compare July 30, 2023 14:52
@gschorcht gschorcht force-pushed the drivers/st77xx_fix_init_single_module branch from d6b06a4 to e50ecaf Compare August 7, 2023 08:01
bors bot added a commit that referenced this pull request Aug 7, 2023
19824: boards/sipeed_longan_nano: separate board definition for Sipeed Longan Nano TFT r=benpicco a=gschorcht

### Contribution description

This PR provides a minimal separate board definition for the Sipeed Longan Nano version with TFT display which is just an extension of `boards/sipeed-longan-nano` with enabled TFT display module.

From the lessons we had to learn with the Kconfig modelling of optional hardware, the TFT version of the Sipeed Longan Nano board has been split off into its own board definition based on the existing Siepeed Longan Nano board.

Commits ba29a5e, 237819e, 6d8b56d and c5faf34 are small cleanups of peripheral configurations and could be split from this PR as follow-up PR (changes +70 -36).

### Testing procedure

Green CI

```
BOARD=sipeed-longan-nano-tft make -j8 -C tests/drivers/st77xx flash
```
should work

### Issues/PRs references

Follow up to PR #19813 and PR #19814
Prerequisite for PR #19825 and PR #19827 

19855: gnrc_static: fix static PID assignment r=benpicco a=benpicco



Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
Co-authored-by: Benjamin Valentin <benjamin.valentin@bht-berlin.de>
@gschorcht gschorcht changed the title drivers/st77xx: impement initialization drivers/st77xx: implement initialization Aug 7, 2023
bors bot added a commit that referenced this pull request Aug 7, 2023
19824: boards/sipeed_longan_nano: separate board definition for Sipeed Longan Nano TFT r=benpicco a=gschorcht

### Contribution description

This PR provides a minimal separate board definition for the Sipeed Longan Nano version with TFT display which is just an extension of `boards/sipeed-longan-nano` with enabled TFT display module.

From the lessons we had to learn with the Kconfig modelling of optional hardware, the TFT version of the Sipeed Longan Nano board has been split off into its own board definition based on the existing Siepeed Longan Nano board.

Commits ba29a5e, 237819e, 6d8b56d and c5faf34 are small cleanups of peripheral configurations and could be split from this PR as follow-up PR (changes +70 -36).

### Testing procedure

Green CI

```
BOARD=sipeed-longan-nano-tft make -j8 -C tests/drivers/st77xx flash
```
should work

### Issues/PRs references

Follow up to PR #19813 and PR #19814
Prerequisite for PR #19825 and PR #19827 

Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
bors bot added a commit that referenced this pull request Aug 7, 2023
19824: boards/sipeed_longan_nano: separate board definition for Sipeed Longan Nano TFT r=benpicco a=gschorcht

### Contribution description

This PR provides a minimal separate board definition for the Sipeed Longan Nano version with TFT display which is just an extension of `boards/sipeed-longan-nano` with enabled TFT display module.

From the lessons we had to learn with the Kconfig modelling of optional hardware, the TFT version of the Sipeed Longan Nano board has been split off into its own board definition based on the existing Siepeed Longan Nano board.

Commits ba29a5e, 237819e, 6d8b56d and c5faf34 are small cleanups of peripheral configurations and could be split from this PR as follow-up PR (changes +70 -36).

### Testing procedure

Green CI

```
BOARD=sipeed-longan-nano-tft make -j8 -C tests/drivers/st77xx flash
```
should work

### Issues/PRs references

Follow up to PR #19813 and PR #19814
Prerequisite for PR #19825 and PR #19827 

19855: gnrc_static: fix static PID assignment r=benpicco a=benpicco



19862: cpu/riscv_common: remove picolibc from blacklisting in CI r=benpicco a=gschorcht

### Contribution description

`picolibc` is now supported by the RISC-V toolchain in `riotdocker`. It is not necessary to blacklist it in CI any longer.

Reference: `picolib` was blacklisted by commit 45270da in PR #15011.

### Testing procedure

1. Green CI
2. Check feature list for CI compilation:
   ```
   BUILD_IN_DOCKER=1 RIOT_CI_BUILD=1 FEATURES_OPTIONAL=picolibc \
   BOARD=sipeed-longan-nano make -j8 -C tests/sys/shell info-modules | grep picolib
   picolibc
   picolibc_stdout_buffered
   picolibc_syscalls_default
   ```

### Issues/PRs references



Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
Co-authored-by: Benjamin Valentin <benjamin.valentin@bht-berlin.de>
@gschorcht gschorcht force-pushed the drivers/st77xx_fix_init_single_module branch from e50ecaf to 8b0be60 Compare August 8, 2023 05:28
@github-actions github-actions bot removed the Area: doc Area: Documentation label Aug 8, 2023
bors bot added a commit that referenced this pull request Sep 6, 2023
19825: drivers: rename st7735 to more generic st77xx r=aabadie a=gschorcht

### Contribution description

This PR provides the following changes:
- renames the driver `st7735` to `st77xx`
- renames the test `st7735` to `st77xx`
- models controller variants as pseudomodules `st7735`, `st7789` and `st7796`
- removes the buggy initialization as a workaround to use reset defaults, see issue #19818 
- adds backward compatibility header files for `ST7735_PARAM_*` symbols
- adds a test board for compilation test of backward compatibility
- updates the corresponding board definitions

The PR should solve the remaining dependency issues in KConfig we had by using `st7735` module for different controller variants. The backward compatibility header files should work for boards that still use `ST7735_PARAM_*` in their board definitions so that the board defintions at user's side use shouldn't be affected.

~To be compilable, the PR includes PR #19824.~

### Testing procedure

- Green CI
- `tests/drivers/disp_dev` and `tests/drivers/st77xx` should still work for all boards using a ST77xx display.
- The PR was already tested for these tests for:
   - [x] `adafruit-pybadge`
   - [x] `esp32s2-lilygo-ttgo-t8`
   - [x] `esp32s3-usb-otg`
   - [x] `sipeed-longan-nano`

### Issues/PRs references

Workaround for issue #19818
Preqruisite for PR #19827 

Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
@gschorcht gschorcht force-pushed the drivers/st77xx_fix_init_single_module branch from 8b0be60 to aec762e Compare September 6, 2023 14:46
@github-actions github-actions bot removed Area: tests Area: tests and testing framework Area: boards Area: Board ports Area: sys Area: System Area: Kconfig Area: Kconfig integration labels Sep 6, 2023
@gschorcht gschorcht removed the State: waiting for other PR State: The PR requires another PR to be merged first label Sep 6, 2023
@aabadie
Copy link
Contributor

aabadie commented Sep 7, 2023

Just tested this on a custom setup (adafruit st7735 breakout connected to a nucleo 64 board). I have something weird: the screen is 128x128 but to get something really perfect I have to set this parameters:

#define ST77XX_PARAM_NUM_LINES  (130U)                          /**< Number of screen lines */
#define ST77XX_PARAM_RGB_CHANNELS   (130U)                      /**< Number of screen rgb channel (height) */

Do you have any idea what could cause this? (on master it crashes...)

For reference this is the configuration I use on the nucleo board:

diff --git a/drivers/st77xx/include/st7735_params.h b/drivers/st77xx/include/st7735_params.h
index a137f1da71..985aff21d1 100644
--- a/drivers/st77xx/include/st7735_params.h
+++ b/drivers/st77xx/include/st7735_params.h
@@ -30,6 +30,25 @@
 
 #include "board.h"
 #include "lcd.h"
+
+/**
+ * @name    Display configuration (not supported yet)
+ * @{
+ */
+#define ST77XX_PARAM_CNTRL      ST77XX_CNTRL_ST7735             /**< ST77xx controller variant */
+#define ST77XX_PARAM_SPI        SPI_DEV(0)                      /**< SPI device */
+#define ST77XX_PARAM_CS         GPIO_PIN(1, 6)                 /**< Chip select pin */
+#define ST77XX_PARAM_DCX        GPIO_PIN(0, 9)                 /**< DCX pin */
+#define ST77XX_PARAM_RST        GPIO_PIN(2, 7)                 /**< Reset pin */
+#define ST77XX_PARAM_NUM_LINES  (130U)                          /**< Number of screen lines */
+#define ST77XX_PARAM_RGB_CHANNELS   (130U)                      /**< Number of screen rgb channel (height) */
+#define ST77XX_PARAM_RGB        (0)                             /**< RGB configuration */
+#define ST77XX_PARAM_INVERTED   (0)                             /**< Inversion configuration */
+#define ST77XX_PARAM_ROTATION   (ST77XX_ROTATION_HORZ_FLIP)     /**< Rotation mode */
+#define LCD_SCREEN_WIDTH        (ST77XX_PARAM_NUM_LINES)        /**< LCD screen width */
+#define LCD_SCREEN_HEIGHT       (ST77XX_PARAM_RGB_CHANNELS)     /**< LCD screen height */
+/** @} */
+
 #include "st7735.h"
 
 #ifdef __cplusplus

@gschorcht
Copy link
Contributor Author

I guess you need to set ST77XX_PARAM_OFFSET_X and ST77XX_PARAM_OFFSET_Y.

Just tested this on a custom setup (adafruit st7735 breakout connected to a nucleo 64 board). I have something weird: the screen is 128x128 but to get something really perfect I have to set this parameters:

I guess you have the Adafruit 1.44" Color TFT LCD Display with MicroSD Card breakout - ST7735R.

I found in the Python Usage page the following comment for this display:

disp = st7735.ST7735R(spi, rotation=270, height=128, x_offset=2, y_offset=3,   # 1.44" ST7735R

So you could try

#define ST77XX_PARAM_NUM_LINES      (128U)    /**< Number of screen lines */
#define ST77XX_PARAM_RGB_CHANNELS   (128U)    /**< Number of screen rgb channel (height) */
#define ST77XX_PARAM_OFFSET_X       (2)
#define ST77XX_PARAM_OFFSET_Y       (3)

@aabadie
Copy link
Contributor

aabadie commented Sep 7, 2023

I guess you need to set ST77XX_PARAM_OFFSET_X and ST77XX_PARAM_OFFSET_Y.

Good point that was that! Just setting #define ST77XX_PARAM_OFFSET_Y (2) is enough, otherwise the top line is not set properly.

Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please squash!

@gschorcht gschorcht force-pushed the drivers/st77xx_fix_init_single_module branch from 12143bd to 7599ce7 Compare September 7, 2023 14:33
@aabadie
Copy link
Contributor

aabadie commented Sep 7, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Sep 7, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit a102560 into RIOT-OS:master Sep 7, 2023
@gschorcht
Copy link
Contributor Author

@aabadie Many thanks.

@gschorcht gschorcht deleted the drivers/st77xx_fix_init_single_module branch September 8, 2023 06:03
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.10 milestone Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

drivers/st7735: faulty driver initialization
5 participants